-
-
Notifications
You must be signed in to change notification settings - Fork 229
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(gdocs): narrative charts component #4295
base: master
Are you sure you want to change the base?
Conversation
Quick links (staging server):
Login:
SVG tester:Number of differences (default views): 0 ✅ Edited: 2024-12-18 21:58:24 UTC |
a985301
to
62e4ec6
Compare
62e4ec6
to
10da0a2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very nice work! No notes on the code :)
Feature wise I think the two things that would be nice to still do is an admin button to hide all controls and a way to see the name, ideally instead of the slug. I think these two features are high enough value to wait for those before shipping, otherwise this is ready!
Okay, this is now finally ready for review 🎉
This PR gets us pretty much all the way there to launch, sans some nice-to-have features.
Related project: #4128
A few notes:
/admin/chartViews
, this is due to an unrelated issue that I'm gonna put a PR up for soonThings still missing
Some things are missing, but afaict all of these are nice-to-haves, really.
See #4128 for a comprehensive list, but a few I'll also list here: